[WellSQL Migration] Migrate `WCNewVisitorStatsModel` to Room #14592

ParaskP7 · 2025-09-08T16:03:12Z

Description

This PR migrates WCNewVisitorStatsModel from WellSQL to Room database.

Reviewers

In addition to having @wzieba as the main "code" reviewer on this change, I added @woocommerce/android-developers to randomly assign a product engineers as an "extra" review (~~@irfano~~ @hichamboushaba), primarily because, both on trunk and this PR branch, I've these questions (⚠️):

OrderStatsRestClient.fetchNewVisitorStats(...): This is where the only place where WCNewVisitorStatsModel get instantiated and its isCustomField is being driven by startDate != null, questions:
- Why is startDate driving this isCustomField field? 🤔
- Due to the above, during my testing, isCustomField was always 1, or else true, just because startDate was always non-null, no matter what option the user chooses from the UI (My store -> Performance -> Calendar icon). Is that expected? 🤔
WCNewVisitorStatsModel: During my testing, when using Custom from the UI (My store -> Performance -> Calendar icon), I noticed that the Visitors (and Conversion) data is unavailable for any date range picked other than a specific "single" day. Is that expected? 🤔

Testing information

`My store` + `Performance`

Navigate on the My store tab and verify that the Performance section is being correctly populated with the expected number of Visitors.
On that Performance section, click on the Calendar icon and select Today. Verify that the section's title changes to something like Today Sep 9, 2025 and that the number of Visitors is updated accordingly.
Similarly, select This Week, This Month and This Year and verify everything is working as expected.
Open App Inspection, check the NewVisitorStatsEntity table, and notice (⚠️) that isCustomField is 1.
Now, select Custom and edit the section's title picking a specific "single" day like Sep 9 - Sep 9, click SAVE. Verify that the section's title changes to something like Custom Sep 9, 2025 - Sep 9, 2025 and that the number of Visitors is updated accordingly.
Then, edit the section's title again, picking any other range, except for a "single" day, like Sep 8 - Sep 9, click SAVE. Verify that the section's title changes to something like Custom Sep 8, 2025 - Sep 9, 2025 and notice (⚠️) that the number of Visitors (and Conversion) is actually unavailable. (see also Reviewers related questions).
Open App Inspection, check the NewVisitorStatsEntity table, and verify that isCustomField is 1.

Images/gif

I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Warning: "'Enum.values()' is recommended to be replaced by 'Enum.entries' since 1.9"

Warning: "Unreachable code"

Warning: "Call chain on a collection type may be simplified"

FYI: This is done in preparation to the subsequent WellSQL to Room table migration.

FYI: This change includes but is not limited to the: - Entity Introduction - DAO Introduction (With Test) - WCAndroidDatabase Migration - Code Adjustments (Store) - Test Adjustments (Store)

Copilot

Pull Request Overview

This PR migrates WCNewVisitorStatsModel from WellSQL to Room database as part of the ongoing database migration effort. The change replaces WellSQL-based persistence with modern Room annotations and DAOs while maintaining the same functionality.

Key changes:

Migrates WCNewVisitorStatsModel from WellSQL annotations to Room entity
Replaces WCVisitorStatsSqlUtils with NewVisitorStatsDao using Room queries
Updates database schema and migration scripts

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
WCNewVisitorStatsModel.kt	Converted from WellSQL `@Table` to Room `@Entity` with composite primary key
NewVisitorStatsDao.kt	New Room DAO replacing WellSQL utility functions
WCStatsStore.kt	Updated to use new DAO instead of SQL utils, made functions suspend
WCVisitorStatsSqlUtils.kt	Deleted - functionality moved to Room DAO
WCAndroidDatabase.kt	Added new entity and DAO registration, version bump
WellSqlConfig.kt	Added migration to drop old WellSQL table
Test files	Updated to use new Room-based approach and test patterns

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

...uxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/dao/NewVisitorStatsDao.kt

Copilot · 2025-09-08T16:03:46Z

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCStatsStore.kt

            }
-        } ?: return mapOf()
+        }
+        return mapOf()


The return statement at line 213 is unreachable because the function always returns from within the rawStats?.let block or the ?: return mapOf() expression on line 212. Remove the unreachable return statement.

Suggested change

return mapOf()

You're right, and that's exactly what I did, actually a bit confused by this comment of yours... 🤔

Yes, the comment is incorrect - I guess LLM wrongly interpreted the inner return (line 203).

A suggestion I have is to refactor for a single return point, but that's not critical.

val visitorMap = rawStats?.let { visitorStatsModel -> val periodIndex = visitorStatsModel.getIndexForField(WCNewVisitorStatsModel.VisitorStatsField.PERIOD) val fieldIndex = visitorStatsModel.getIndexForField(WCNewVisitorStatsModel.VisitorStatsField.VISITORS) if (periodIndex != -1 && fieldIndex != -1) { getVisitorsMap( periodIndex = periodIndex, fieldIndex = fieldIndex, dataList = visitorStatsModel.dataList ) } else { emptyMap() } } return visitorMap ?: emptyMap()

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCStatsStore.kt

dangermattic · 2025-09-08T16:04:10Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-09-08T16:14:39Z

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`2e11297`
	Direct Download	`woocommerce-wear-prototype-build-pr14592-2e11297.apk`

wpmobilebot · 2025-09-08T16:25:51Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`2e11297`
	Direct Download	`woocommerce-prototype-build-pr14592-2e11297.apk`

codecov-commenter · 2025-09-08T22:40:01Z

Codecov Report

❌ Patch coverage is 6.49351% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.38%. Comparing base (92303be) to head (2e11297).

Files with missing lines	Patch %	Lines
.../org/wordpress/android/fluxc/store/WCStatsStore.kt	9.80%	46 Missing ⚠️
...ress/android/fluxc/model/WCNewVisitorStatsModel.kt	0.00%	9 Missing ⚠️
...k/rest/wpcom/wc/orderstats/OrderStatsRestClient.kt	0.00%	9 Missing ⚠️
...ndroid/fluxc/persistence/dao/NewVisitorStatsDao.kt	0.00%	4 Missing ⚠️
...rdpress/android/fluxc/persistence/WellSqlConfig.kt	0.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14592      +/-   ##
============================================
+ Coverage     38.35%   38.38%   +0.02%     
  Complexity     9715     9715              
============================================
  Files          2058     2058              
  Lines        115287   115226      -61     
  Branches      15341    15337       -4     
============================================
+ Hits          44220    44224       +4     
+ Misses        66970    66906      -64     
+ Partials       4097     4096       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…to migrate/wc-new-visitor-stats-model-to-room

irfano · 2025-09-09T16:30:07Z

I unassigned myself. Since I’ll be AFK until September 19, can you take a look at this, @JorgeMucientes or @hichamboushaba?

ParaskP7 · 2025-09-10T08:15:05Z

I unassigned myself. Since I’ll be AFK until September 19...

Oh, thanks @irfano and apologies, I (or algorithm) wasn't aware you're on AFK. 🫣

FYI: I am not sure if you had the change to use this AFK/Github Busy Status Sync (paaHJt-8Ri-p2) yet, it might help you folks in situations like this in the future. Cc @woocommerce/android-developers for further awareness. 🙏

...can you take a look at this, @JorgeMucientes or @hichamboushaba?

Since I've assign @JorgeMucientes on this previous #14520 PR of mine already, I'll go ahead and assign you @hichamboushaba on this one, let me know how you feel and I'll find an alternative. 🙏

wzieba

LGTM!

I don't know the answers on the domain of the Stats, so I'll leave this to the product team. I also couldn't perform detailed tests due to log in issues (internal details: p1757495577372719-slack-CGPNUU63E ) but I don't want to keep this PR on hold because of this.

wzieba · 2025-09-09T11:38:49Z

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCStatsStore.kt

            }
-        } ?: return mapOf()
+        }
+        return mapOf()


Yes, the comment is incorrect - I guess LLM wrongly interpreted the inner return (line 203).

A suggestion I have is to refactor for a single return point, but that's not critical.

val visitorMap = rawStats?.let { visitorStatsModel -> val periodIndex = visitorStatsModel.getIndexForField(WCNewVisitorStatsModel.VisitorStatsField.PERIOD) val fieldIndex = visitorStatsModel.getIndexForField(WCNewVisitorStatsModel.VisitorStatsField.VISITORS) if (periodIndex != -1 && fieldIndex != -1) { getVisitorsMap( periodIndex = periodIndex, fieldIndex = fieldIndex, dataList = visitorStatsModel.dataList ) } else { emptyMap() } } return visitorMap ?: emptyMap()

wzieba · 2025-09-09T11:44:03Z

config/detekt/baseline.xml

    <ID>UnitTestNamingRule:WCShippingLabelStoreTest.kt$WCShippingLabelStoreTest$@Test fun `verify shipping address`()</ID>
    <ID>UnitTestNamingRule:WCStatsSqlUtilsTest.kt$WCStatsSqlUtilsTest$@Test @Suppress("LongMethod") fun testGetRawRevenueStatsForSiteAndUnit()</ID>
    <ID>UnitTestNamingRule:WCStatsSqlUtilsTest.kt$WCStatsSqlUtilsTest$@Test @Suppress("LongMethod") fun testSimpleInsertionAndRetrievalOfRevenueStats()</ID>
-    <ID>UnitTestNamingRule:WCStatsStoreTest.kt$WCStatsStoreTest$@Suppress("LongMethod") @Test fun testGetQuantityForMonths()</ID>


Nice work on this (here and previous PRs) - a good reminder to look at the baseline (Detekt or Lint) when working on technical debt 🏅

ParaskP7 · 2025-09-10T09:41:04Z

Thanks for the review @wzieba (🙇) , I'll wait for another review from the product team, plus maybe you being able to successfully test this afterwards, and I'll then proceed with applying any of the review suggestions (like this one). 👍

hichamboushaba · 2025-09-10T15:00:01Z

I'll go ahead and assign you @hichamboushaba on this one, let me know how you feel and I'll find an alternative. 🙏

Sounds good @ParaskP7, but I hope it's fine not to review this until tomorrow 🙂

ParaskP7 · 2025-09-10T15:06:56Z

Sounds good @ParaskP7, but I hope it's fine not to review this until tomorrow 🙂

Totally fine @hichamboushaba , many thanks + appreciated, and no rush, no rush at all! 🙇 ❤️

hichamboushaba

Nice work @ParaskP7 👏, this works as expected, and I'll try to answer your questions below:

OrderStatsRestClient.fetchNewVisitorStats(...): This is where the only place where WCNewVisitorStatsModel get instantiated and its isCustomField is being driven by startDate != null, questions:

Why is startDate driving this isCustomField field? 🤔

Due to the above, during my testing, isCustomField was always 1, or else true, just because startDate was always non-null, no matter what option the user chooses from the UI (My store -> Performance -> Calendar icon). Is that expected? 🤔

I also didn't know, but after looking at the history of this field, I understand now, back in 2019 when this table was added, the app supported a "custom" date option in stats graph (that option was removed later), and the way the team worked on this was that they treated the granularity option as a date (so StatsGranularity.DAYS was equivalent to "Today") unless the startDate was specified, startDate was specified only for the custom date option in the graph.
This was confusing, and we got rid of this when we brought back the "Custom" date option last year, and we changed the logic so that startDate was always required.
I overlooked the column isCustomField of the DB when I worked on this, otherwise I should have deleted it as it's always true now.
Now whether you should delete it or not, I'll leave it up to you to decide this 🙂.

WCNewVisitorStatsModel: During my testing, when using Custom from the UI (My store -> Performance -> Calendar icon), I noticed that the Visitors (and Conversion) data is unavailable for any date range picked other than a specific "single" day. Is that expected? 🤔

Yes, it's expected, but this has nothing to do with this migration, the Visitors count that's shown on the graph when no entry is selected is driven by the VisitorSummaryStatsEntity, and this used Room since it was added.

Please let me know if you have any other questions, but the main point here is that this is ready for merge unless you want to delete the isCustomField column.

ParaskP7 · 2025-09-11T15:13:43Z

👋 @hichamboushaba and thanks for reviewing/testing this, as well as answering my questions, much appreciated! 🙇 ❤️

Nice work @ParaskP7 👏, this works as expected, and I'll try to answer your questions below:

Thank you for digging as deep to answer my question! 🙏

Please let me know if you have any other questions, but the main point here is that this is ready for merge unless you want to delete the isCustomField column.

Yea, I think it might be better (an opportunity) for us to delete this unused isCustomField column (should be hard), then make this whole solution, store/rest/persistence that bit less complicated. And if not for anything else, just to avoid having this discussion again in the future when someone else will try and touch this logic.

WILL DO 🏃 💨💨💨

…to migrate/wc-new-visitor-stats-model-to-room # Conflicts: # libs/fluxc-plugin/schemas/org.wordpress.android.fluxc.persistence.WCAndroidDatabase/64.json

Due to 146ebde a merge conflict appeared which got solved via 781b01b, with the main '64.json' file, but also a hidden merge conflict on 'WCAndroidDatabase', which includes: - const val WC_DATABASE_VERSION = 65 - AutoMigration(from = 64, to = 65), I accepted the current version of '64.json' and created a new one, the '65.json' because the database version for this exact change has now been updated to '65' (from '64').

PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14592#pullrequestreview-3212230060

ParaskP7 · 2025-09-12T12:43:03Z

WILL DO 🏃 💨💨💨

FYI: This is now done @hichamboushaba (cc @wzieba): e1694cd (mainly)

Please take another look and let me know if everything looks and works as expected, thank you much! 🙏

PS: No rush! 🆗

Copilot

Pull Request Overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCStatsStore.kt

hichamboushaba

Nice work @ParaskP7, thanks so much for the code cleanup

ParaskP7 · 2025-09-12T14:59:10Z

Nice work @ParaskP7, thanks so much for the code cleanup

Awesome @hichamboushaba , thanks so much for yet another code review/testing session on that! 🙇 ❤️ 🚀

FYI: I'll merge this first thing on Monday.

ParaskP7 added 11 commits September 8, 2025 18:51

Refactor: Optimize imports on wc stats store

540e365

Analysis: Replace order stats error type with enum entries

13a299a

Warning: "'Enum.values()' is recommended to be replaced by 'Enum.entries' since 1.9"

Analysis: Fix unreachable code on get new visitor stats

eafaa52

Warning: "Unreachable code"

Analysis: Merge call chain to associate on get xyz stats

587a818

Warning: "Call chain on a collection type may be simplified"

Refactor: Reformat wc stats store (test)

10c0c88

Refactor: Make getNewVisitorStats suspend functions

dafe3c2

FYI: This is done in preparation to the subsequent WellSQL to Room table migration.

Cleanup: Remove unnecessary rows affected from on wc stats changed

a9c67c1

FYI: This is done in preparation to the subsequent WellSQL to Room table migration.

Refactor: Optimize imports on wc stats store test (and fix typo)

13bb637

Test: Move wc stats test utils and wc stats store test to fluxc-plugin

f65dfe2

FYI: This is done in preparation to the subsequent WellSQL to Room table migration.

Migrate: 'WCNewVisitorStatsModel' from wellsql to room

4f5a86e

FYI: This change includes but is not limited to the: - Entity Introduction - DAO Introduction (With Test) - WCAndroidDatabase Migration - Code Adjustments (Store) - Test Adjustments (Store)

Db: Drop 'WCNewVisitorStatsModel' table from 'wp-fluxc'

10fbbfb

ParaskP7 added this to the 23.3 milestone Sep 8, 2025

ParaskP7 requested a review from Copilot September 8, 2025 16:03

ParaskP7 added the type: technical debt Represents or solves tech debt of the project. label Sep 8, 2025

Copilot AI reviewed Sep 8, 2025

View reviewed changes

Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…

06cae58

…to migrate/wc-new-visitor-stats-model-to-room

ParaskP7 marked this pull request as ready for review September 9, 2025 11:14

ParaskP7 requested review from a team, irfano and wzieba and removed request for a team September 9, 2025 11:14

irfano removed their request for review September 9, 2025 16:28

ParaskP7 requested a review from hichamboushaba September 10, 2025 08:15

wzieba approved these changes Sep 10, 2025

View reviewed changes

hichamboushaba self-assigned this Sep 11, 2025

hichamboushaba approved these changes Sep 11, 2025

View reviewed changes

ParaskP7 added 5 commits September 12, 2025 12:34

Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…

781b01b

…to migrate/wc-new-visitor-stats-model-to-room # Conflicts: # libs/fluxc-plugin/schemas/org.wordpress.android.fluxc.persistence.WCAndroidDatabase/64.json

Cleanup: Remove unnecessary is custom field logic from new visitor stats

e1694cd

PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14592#pullrequestreview-3212230060

Cleanup: Remove leftover custom keywork from new visitor stats dao

ef6197e

Refactor: Rename insert/update to replace stat on new visitor stats dao

2e11297

ParaskP7 requested review from Copilot, hichamboushaba and wzieba September 12, 2025 12:43

Copilot AI reviewed Sep 12, 2025

View reviewed changes

hichamboushaba approved these changes Sep 12, 2025

View reviewed changes

ParaskP7 merged commit 603ef16 into trunk Sep 15, 2025
17 checks passed

ParaskP7 deleted the migrate/wc-new-visitor-stats-model-to-room branch September 15, 2025 07:08

[WellSQL Migration] Migrate WCNewVisitorStatsModel to Room #14592

[WellSQL Migration] Migrate WCNewVisitorStatsModel to Room #14592

Uh oh!

Conversation

ParaskP7 commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewers

Testing information

My store + Performance

Images/gif

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

ParaskP7 Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wzieba Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dangermattic commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

irfano commented Sep 9, 2025

Uh oh!

ParaskP7 commented Sep 10, 2025

Uh oh!

wzieba left a comment

Choose a reason for hiding this comment

Uh oh!

wzieba Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

wzieba Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

ParaskP7 commented Sep 10, 2025

Uh oh!

hichamboushaba commented Sep 10, 2025

Uh oh!

ParaskP7 commented Sep 10, 2025

Uh oh!

hichamboushaba left a comment

Choose a reason for hiding this comment

Uh oh!

ParaskP7 commented Sep 11, 2025

Uh oh!

ParaskP7 commented Sep 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hichamboushaba left a comment

Choose a reason for hiding this comment

Uh oh!

ParaskP7 commented Sep 12, 2025

Uh oh!

[WellSQL Migration] Migrate `WCNewVisitorStatsModel` to Room #14592

[WellSQL Migration] Migrate `WCNewVisitorStatsModel` to Room #14592

ParaskP7 commented Sep 8, 2025 •

edited

Loading

`My store` + `Performance`

ParaskP7 Sep 8, 2025 •

edited

Loading

dangermattic commented Sep 8, 2025 •

edited

Loading

wpmobilebot commented Sep 8, 2025 •

edited

Loading

wpmobilebot commented Sep 8, 2025 •

edited

Loading

codecov-commenter commented Sep 8, 2025 •

edited

Loading